Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readthedocs: remove sys pkgs; use current ADIOS2 #3743

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

vicentebolea
Copy link
Collaborator

@vicentebolea vicentebolea commented Aug 7, 2023

Readthedocs is phasing out system packages as shown in the screenshot below. I am also disabling conda environment in the readthdocs build since it is redundant and we do alreay install those deps with pip. Also we do not need to install adios2 from conda to build the docs.

Screenshot from 2023-08-07 13-22-07

@vicentebolea vicentebolea force-pushed the readthedocs-use-pip branch 6 times, most recently from 6b97f9d to 4ecc1bf Compare August 7, 2023 17:19
@scottwittenburg
Copy link
Collaborator

The "Details" link for the the readthedocs build takes me to a page that doesn't look right. It's kind of like the generated html without any images or stylesheets, where I was expecting a link to the details of running the readthedocs commands. I don't know much about rtd, so I'm not sure if this is expected, or a temporary situation, or some issue caused by this PR.

@vicentebolea
Copy link
Collaborator Author

The "Details" link for the the readthedocs build takes me to a page that doesn't look right. It's kind of like the generated html without any images or stylesheets, where I was expecting a link to the details of running the readthedocs commands. I don't know much about rtd, so I'm not sure if this is expected, or a temporary situation, or some issue caused by this PR.

That is very odd, I Can see it displayed well. Thanks for pointing this out, which browser are you using?

@scottwittenburg
Copy link
Collaborator

which browser are you using?

I'm using firefox on linux

@vicentebolea
Copy link
Collaborator Author

hmm same as I, could you post an screenshot, I cannot reproduce it

@scottwittenburg
Copy link
Collaborator

readthedocs_link

@vicentebolea
Copy link
Collaborator Author

@scottwittenburg I cannot reproduce this, I open it in the phone/chrome/firefox but I get the correct theme:

https://adios2--3743.org.readthedocs.build/en/3743/

Maybe something with your browser

@vicentebolea
Copy link
Collaborator Author

Can you open this correctly: https://adios2.readthedocs.io/en/v2.9.1/

@vicentebolea
Copy link
Collaborator Author

Screenshot from 2023-08-07 14-51-51

@williamfgc
Copy link
Contributor

FYI, we used conda at that time 'cause the breathe, sphinx rtd packages needed a adios2 build to translate the pybind11 doxygen md annotated code in pyglue.cpp to html.

@vicentebolea
Copy link
Collaborator Author

FYI, we used conda at that time 'cause the breathe, sphinx rtd packages needed a adios2 build to translate the pybind11 doxygen md annotated code in pyglue.cpp to html.

Many thanks @williamfgc for pointing this out. So I understand that we need an ADIOS2 build in order to generate the Python binding full api page with pybind11, if that is the case I think that we should then stick to conda then. A problem that I can see is that we will be always pulling the latest release from Conda rather than master branch thus changes in these py11glue files will not get reflected right away, but 🤷 I cannot see a reasonable way to resolve this.

@williamfgc
Copy link
Contributor

I cannot see a reasonable way to resolve this.

Thanks @vicentebolea for revisiting this. This is far from perfect and seemed like a chicken-and-egg problem. Perhaps an adios2 wheels would solve this? Mostly asking as I am not that familiar.

@vicentebolea vicentebolea force-pushed the readthedocs-use-pip branch 10 times, most recently from 3a90548 to 9c331dd Compare August 7, 2023 22:30
@vicentebolea
Copy link
Collaborator Author

@williamfgc I tried to build a minimal ADIOS2 build inside readthedocs and it seems to be sufficient for building the readthedocs site.

- Solely use Conda to install deps
@vicentebolea vicentebolea changed the title readthedocs: solely use requirements.txt readthedocs: remove sys pkgs; use current ADIOS2 Aug 7, 2023
@vicentebolea
Copy link
Collaborator Author

@scottwittenburg I was able to build a minimal adios in readthedocs without much trouble/time

Copy link
Collaborator

@scottwittenburg scottwittenburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @vicentebolea!

@vicentebolea vicentebolea enabled auto-merge August 8, 2023 19:26
@vicentebolea vicentebolea merged commit afbe6d6 into ornladios:master Aug 8, 2023
@vicentebolea vicentebolea deleted the readthedocs-use-pip branch August 8, 2023 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants